-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix: Apply text-disabled to labels and hint blocks when components are disabled #194
Conversation
🦋 Changeset detectedLatest commit: cf944f6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Preview URLsEnv: preview |
57455b8
to
d53d3ce
Compare
d53d3ce
to
cbd1bb1
Compare
@@ -16,7 +16,8 @@ | |||
) | |||
}} | |||
<legend | |||
class="type-md-tight text-body-and-labels flex items-center gap-1.5" | |||
class="type-md-tight flex items-center gap-1.5 | |||
{{if @isDisabled 'text-disabled' 'text-body-and-labels'}}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be tempting to make a GroupLegend
component or something, that's essentially Label
but renders a <legend
and can be used here and with the radio-group; however, I punted on this for now. Happy to do it in a follow up if folks think it'd be worth it though.
@@ -15,6 +15,7 @@ module('Integration | Component | Fields | CheckboxField', function (hooks) { | |||
</template>); | |||
|
|||
assert.dom('[data-label]').hasText('Label'); | |||
assert.dom('[data-label]').hasClass('text-titles-and-attributes'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've got mixed feelings on asserting class names in tests, as I'd prefer visual regression testing instead; however, I want to make sure we don't regress until we get something like Percy added.
Note 2: Radio and checkbox labels use text-titles-and-attributes
instead of text-body-and-labels
intentionally by design
@@ -57,6 +57,8 @@ module('Integration | Component | Fields | FileInput', function (hooks) { | |||
</template>); | |||
|
|||
assert.dom('[data-label]').hasText('Label'); | |||
assert.dom('label').hasClass('text-body-and-labels'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FileInput is a bit different than the others, where because of the markup, data-label
is actually a nested element within <label
. Due to that, I'm targeting the label
tag directly here instead of using [data-label]
cbd1bb1
to
347e8c5
Compare
347e8c5
to
cf944f6
Compare
(Downstream of #193 for DX improvement reasons)
🚀 Description
Adds disabled styling to labels and hint blocks when
@isDisabled
.🔬 How to Test
View the following URLs and verify
text-disabled
is applied📸 Images/Videos of Functionality